Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators by Copilot · Pull Request #64278 · dotnet/aspnetcore

Copilot · 2025-11-07T18:29:20Z

Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators

You've read the Contributor Guide and Code of Conduct.
You've included unit or integration tests for your change, where applicable.
You've included inline docs for your change, where applicable.
There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Clarify RefreshSignInAsync requirements and fix inconsistent null checks

Description

The RefreshSignInAsync documentation did not state that users must already be signed in. The method also used inconsistent null-conditional operators—checking auth.Succeeded directly but then using auth?.Properties despite AuthenticateResult.Succeeded guaranteeing non-null properties.

Changes:

Added <remarks> to documentation clarifying user must be signed in and user ID must match
Removed ?. operators on auth.Principal and auth.Properties after auth.Succeeded validation (lines 211, 212, 228)
Retained ?. operators in failure branches where properties may be null

// Before: inconsistent null handling
var authenticationMethod = auth.Principal?.FindFirst(ClaimTypes.AuthenticationMethod);
return (true, auth.Properties?.IsPersistent ?? false);

// After: leverages [MemberNotNullWhen(true)] guarantees
var authenticationMethod = auth.Principal.FindFirst(ClaimTypes.AuthenticationMethod);
return (true, auth.Properties.IsPersistent);

Fixes #{issue number}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

cdn.jsdelivr.net
- Triggering command: /home/REDACTED/work/aspnetcore/aspnetcore/.dotnet/dotnet exec --depsfile /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.deps.json --runtimeconfig /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.runtimeconfig.json /home/REDACTED/.nuget/packages/xunit.REDACTED.console/2.9.2/tools/netcoreapp2.0/xunit.console.dll /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.dll -noautoreporters -xml /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.xml -html /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.html -notrait Quarantined=true -nocolor (dns block)
cdnjs.cloudflare.com
- Triggering command: /home/REDACTED/work/aspnetcore/aspnetcore/.dotnet/dotnet exec --depsfile /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.deps.json --runtimeconfig /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.runtimeconfig.json /home/REDACTED/.nuget/packages/xunit.REDACTED.console/2.9.2/tools/netcoreapp2.0/xunit.console.dll /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.dll -noautoreporters -xml /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.xml -html /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.html -notrait Quarantined=true -nocolor (dns block)
stackpath.bootstrapcdn.com
- Triggering command: /home/REDACTED/work/aspnetcore/aspnetcore/.dotnet/dotnet exec --depsfile /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.deps.json --runtimeconfig /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.runtimeconfig.json /home/REDACTED/.nuget/packages/xunit.REDACTED.console/2.9.2/tools/netcoreapp2.0/xunit.console.dll /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Identity.Test/Debug/net10.0/Microsoft.AspNetCore.Identity.Test.dll -noautoreporters -xml /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.xml -html /home/REDACTED/work/aspnetcore/aspnetcore/artifacts/TestResults/Debug/Microsoft.AspNetCore.Identity.Test_net10.0_x64.html -notrait Quarantined=true -nocolor (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

For SignInManager.RefreshSignInAsync,

Could you please update the docs for the method as well.
The current docs do not mention anywhere that the user must already be logged in!
In the old code, if the user wasn't sign-in, this worked like a normal call to "SignInAsync".

This "patch" broke some edge-cases in our productive environment when azure updated dotnet for our webapps.

Suggested docs:
    /// <summary>
    /// Refresh the sign with the specified <paramref name="user"/>, whilst preserving the existing
    /// AuthenticationProperties of the current signed-in user like rememberMe, as an asynchronous operation.
    /// </summary>
    /// <remarks> User must already be signed in, and user id must match existing user. </remarks>
    /// <param name="user">The user to refresh the sign-in for.</param>
    /// <returns>The task object representing the asynchronous operation.</returns>
Useless null checks: on line 180, 181 and 196 you check for auth?.
But on line 165 you access auth.Succeeded so if auth could be null this would throw already on line 165. Can we just add a null check during the first if, and if it's null treat the user as unauthenticated and log an error, and then remove the useless null checks later in the method.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

dotnet-policy-service · 2025-11-07T18:29:37Z

Greetings human! You've submitted a PR that modifies code that is shared with https://github.com/dotnet/runtime . Please make sure you synchronize this code with the changes in that repo!

- Updated XML documentation to clearly state user must be signed in - Added explicit null check for auth result at the beginning - Removed unnecessary null-conditional operators after auth.Succeeded check - All existing tests pass Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>

AuthenticateAsync guarantees to return non-null result, so the explicit null check is not needed. The code now only removes unnecessary null- conditional operators after confirming auth.Succeeded is true. Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>

Update method reference to include nullable parameter annotation (string?) Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>

Copilot

Pull Request Overview

This PR improves the RefreshSignInAsync method documentation and removes unnecessary null-conditional operators that are redundant after null-safety checks.

Key changes:

Added <remarks> section to clarify that RefreshSignInAsync requires the user to already be signed in with matching user ID
Removed unnecessary null-conditional operators (?.) on auth.Principal and auth.Properties after auth.Succeeded validation, leveraging the [MemberNotNullWhen(true)] attribute guarantees

Comments suppressed due to low confidence (1)

src/Identity/Core/src/SignInManager.cs:196

The null-conditional operator ?. on auth.Principal is unnecessary here. When !auth.Succeeded is false (meaning auth.Succeeded is true), the [MemberNotNullWhen(true)] attribute guarantees that auth.Principal is non-null. Consider changing to auth.Principal.Identity?.IsAuthenticated != true for consistency with the other changes in this PR.

        if (!auth.Succeeded || auth.Principal?.Identity?.IsAuthenticated != true)

src/Identity/Core/src/SignInManager.cs

dotnet-policy-service · 2025-11-20T06:04:54Z

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

Initial plan

45900c9

Copilot AI assigned Copilot and halter73 Nov 7, 2025

dotnet-policy-service bot added the Attention: Shared Code Modified label Nov 7, 2025

Copilot started work on behalf of halter73 November 7, 2025 18:29 View session

Copilot AI and others added 3 commits November 7, 2025 18:50

Fix XML documentation method reference

6b55648

Update method reference to include nullable parameter annotation (string?) Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Update SignInManager.RefreshSignInAsync documentation~~ Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators Nov 7, 2025

Copilot AI requested a review from halter73 November 7, 2025 19:00

Copilot finished work on behalf of halter73 November 7, 2025 19:00

build-analysis bot mentioned this pull request Nov 7, 2025

Known Build Error: error : Cannot create a file when that file already exists. #61178

Open

halter73 marked this pull request as ready for review November 11, 2025 23:05

Copilot AI review requested due to automatic review settings November 11, 2025 23:05

halter73 approved these changes Nov 11, 2025

View reviewed changes

Copilot started reviewing on behalf of halter73 November 11, 2025 23:06 View session

Copilot finished reviewing on behalf of halter73 November 11, 2025 23:07

Copilot AI reviewed Nov 11, 2025

View reviewed changes

halter73 reviewed Nov 12, 2025

View reviewed changes

src/Identity/Core/src/SignInManager.cs Outdated Show resolved Hide resolved

Update src/Identity/Core/src/SignInManager.cs

f26612f

dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Nov 20, 2025

github-actions bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Feb 10, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators#64278

Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators#64278
Copilot wants to merge 5 commits intomainfrom
copilot/update-signinmanager-docs

Copilot AI commented Nov 7, 2025 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

dotnet-policy-service bot commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update RefreshSignInAsync documentation and remove unnecessary null-conditional operators

Description

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

dotnet-policy-service bot commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

dotnet-policy-service bot commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Nov 7, 2025 •

edited

Loading